Allow $0 as valid card limit when editing Expensify Card#90540
Allow $0 as valid card limit when editing Expensify Card#90540MelvinBot wants to merge 4 commits into
Conversation
The validation in DynamicExpensifyCardLimitPage used !Number(values.limit)
to check for invalid amounts. Since Number('0') is 0 and !0 is true,
a $0 limit was always rejected as invalid. This prevented editing cards
whose limit was set to $0 in Expensify Classic.
Changed the check to Number.isNaN(Number(values.limit)) which correctly
allows 0 while still rejecting non-numeric input.
Co-authored-by: Georgia Monahan <grgia@users.noreply.github.com>
Codecov Report❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.
|
|
🚧 @grgia has triggered a test Expensify/App build. You can view the workflow run here. |
This comment has been minimized.
This comment has been minimized.
|
@sobitneupane Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
|
@grgia The issue is still reproducible on my end. Steps To reproduce:
Screen.Recording.2026-05-16.at.18.07.36.mov |
|
This is for another card. When I try to edit limit and set it to non-zero value, it disappears from the list. I suspect this is because we only show inactive cards with a $0 limit. Screen.Recording.2026-05-16.at.18.40.53.mov |
|
Here are my findings:
Proposed change:
function filterAllInactiveCards(cards: CardList | undefined, includeDeactivated = false) {
if (!cards) {
return {};
}
const closedStates = new Set<number>([CONST.EXPENSIFY_CARD.STATE.CLOSED, CONST.EXPENSIFY_CARD.STATE.STATE_DEACTIVATED]);
return filterObject(cards, (_key, card) => {
const isAdminZeroedExpensifyCard = isExpensifyCard(card) && isCardWithCustomZeroLimit(card);
if (card.state === CONST.EXPENSIFY_CARD.STATE.STATE_SUSPENDED) {
- return !!card.nameValuePairs?.frozen || (includeDeactivated && isAdminZeroedExpensifyCard);
+ return !!card.nameValuePairs?.frozen || includeDeactivated
}
- if (card.state === CONST.EXPENSIFY_CARD.STATE.STATE_DEACTIVATED) {
- return includeDeactivated && isAdminZeroedExpensifyCard;
- }
return !closedStates.has(card.state);
});
} |
This comment was marked as resolved.
This comment was marked as resolved.
…activated cards When includeDeactivated is true, show all suspended cards (not just zero-limit ones) so admins can view and edit them. Remove the deactivated card path entirely since the server rejects limit updates on STATE_DEACTIVATED cards with a 403 error. Co-authored-by: Georgia Monahan <grgia@users.noreply.github.com>
|
🚧 @grgia has triggered a test Expensify/App build. You can view the workflow run here. |
|
Updated the PR with both changes:
|
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
…rds behavior Tests were asserting old behavior where only admin-zeroed suspended cards were kept. Updated to reflect the new logic: all suspended cards are kept when includeDeactivated is true, and STATE_DEACTIVATED cards are always excluded since the server rejects limit updates on them. Co-authored-by: Georgia Monahan <grgia@users.noreply.github.com>
|
Fixed the failing |
|
The failing Failure: Why it's unrelated: This PR only modifies |
|
@MelvinBot merge main |
…lidation Co-authored-by: Georgia Monahan <grgia@users.noreply.github.com>
|
Merged main into this branch successfully. |
|
@sobitneupane latest branch works on my card test- can you check with yours? |
|
If it still doesn't feel free to stop there |
Explanation of Change
Two fixes for Expensify Card management when cards have a $0 limit set from OldDot:
1.
filterAllInactiveCardsinCardUtils.ts— WhenincludeDeactivatedis true (workspace card list), show all suspended cards so admins can view and edit them, not just zero-limit ones. Also removes the code path that returnedSTATE_DEACTIVATEDcards, since the server rejects limit updates on deactivated cards with a 403 error — those cards should never appear in the editable card list.2. Validator in
DynamicExpensifyCardLimitPage.tsx— Changed!Number(values.limit)toNumber.isNaN(Number(values.limit))so that $0 is accepted as a valid limit. A suspended card with a current limit of $0 still needs to load the edit form without tripping "invalid amount".Fixed Issues
$ #90487
Tests
// TODO: The human co-author must fill out the tests you ran before marking this PR as "ready for review"
// Please describe what tests you performed that validates your changed worked.
Offline tests
N/A — validation is purely client-side with no network dependency.
QA Steps
// TODO: The human co-author must fill out the QA tests you ran before marking this PR as "ready for review".
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari